feat: resharding - tests for missing chunks #10052
Conversation
wacban
force-pushed
the
waclaw-resharding-tests
branch
from
a4dd5fb
to
c6e8702
Compare
| for chunk in chunks.iter() { | ||
| assert_ne!(chunk.height_included(), last_block.header().height()); | ||
| } |
There was a problem hiding this comment.
This check was overly ambitious for V2. When testing the V1 resharding, it's all or nothing when it comes to chunk production. In the V2 resharding, which happens to include ChunkOnlyProducers, chunks in a block can be produced by different chunk producers and so some can be present and some missing.
I changed it to have the same condition for both cases, when the shard layout is the same and when it is different. The condition now is that the shards with ids target_shard_ids are missing in all blocks from block_hash to last_block_hash_with_empty_chunk.
| if path.exists() { | ||
| std::fs::remove_dir_all(&path)? | ||
| } | ||
| Ok(()) |
There was a problem hiding this comment.
not relevant to the rest of the PR, without this check it would print some nasty errors
chain/client/src/client.rs
Outdated
| debug!(target: "client", "{:?} Producing block at height {}, parent {} @ {}, {} new chunks: {:?}", validator_signer.validator_id(), | ||
| next_height, prev.height(), format_hash(head.last_block_hash), new_chunks.len(), new_chunks.keys().sorted()); |
There was a problem hiding this comment.
not relevant to the rest of the PR, but useful debug info
There was a problem hiding this comment.
Please make it friendly to grafana tempo:
debug!(target: "client", next_height, prev_height = prev.height(), parent, ..., "Producing block");
chain/client/src/client.rs
Outdated
| debug!(target: "client", "{:?} Producing block at height {}, parent {} @ {}, {} new chunks: {:?}", validator_signer.validator_id(), | ||
| next_height, prev.height(), format_hash(head.last_block_hash), new_chunks.len(), new_chunks.keys().sorted()); |
There was a problem hiding this comment.
Please make it friendly to grafana tempo:
debug!(target: "client", next_height, prev_height = prev.height(), parent, ..., "Producing block");
|
Oh great, I ran into this couple days ago and almost started thinking about a fix. |
|
Not sure what exactly is "this" but I'm happy to have helped :) |
|
I want to add a bunch of similar tests for which no resharding is happening. It is pretty convenient check of outgoing receipts handling for stateless validation. But because of broken override it didn't work for latest protocol versions before, now it should work without patches. |
get_next_block_hash_with_new_chunkthat didn't work for the V2 resharding